-
Notifications
You must be signed in to change notification settings - Fork 29
Refresh AI job UI #8873
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Refresh AI job UI #8873
Conversation
📝 WalkthroughWalkthroughBackend job-cost mapping extended for new JobCommand variants; frontend replaces the modal-based AI-job UX with a drawer-based system—removing modal components and adding drawer tabs, contexts, selectors, validators, credit UI, training/run/alignment pages, helpers, import/path updates, styles, docs, and admin API/type renames. Changes
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
docs/automation/ai_training.md (1)
25-28: Doc still references the old dialog UI.The new flow opens a drawer with a “Model Training” tab. Leaving “dialog” here misleads users. Please update the wording to match the drawer-based UI.
-## Configuring the Training -To start a training, click on the `AI Analysis` button in the toolbar and select `Train AI model` from the dropdown menu. -This will open a dialog where you can configure and start your training job. +## Configuring the Training +To start training, click the `AI Analysis` button in the toolbar and choose `Train AI model`. +This opens a drawer with a `Model Training` tab where you can configure and start your training job.
🧹 Nitpick comments (1)
docs/automation/ai_training.md (1)
56-58: Fix the typo in the closing paragraph.“Find and overview” should be “find an overview.”
-Once the training is finished, you can find and overview of all your trained model from the `Admin` >`AI Models` page in the navbar. +Once the training is finished, you can find an overview of all your trained models on the `Admin` > `AI Models` page in the navbar.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/automation/ai_training.md(1 hunks)docs/automation/alignment.md(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/automation/alignment.md
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (4)
frontend/javascripts/viewer/view/ai_jobs/hooks/fetch_annotation_infos.ts (4)
65-79: Stop pushing to (userBoundingBoxes || []); mutate the actual arrayGuard above ensures userBoundingBoxes is an array, so drop the fallback to avoid mutating a temporary. Also return the variable directly.
- if (annotation.task?.boundingBox) { - const existingIds = (userBoundingBoxes || []).map(({ id }) => id); - const largestId = existingIds.length > 0 ? Math.max(...existingIds) : -1; - (userBoundingBoxes || []).push({ + if (annotation.task?.boundingBox) { + const existingIds = userBoundingBoxes.map(({ id }) => id); + const largestId = existingIds.length > 0 ? Math.max(...existingIds) : -1; + userBoundingBoxes.push({ name: "Task Bounding Box", boundingBox: Utils.computeBoundingBoxFromBoundingBoxObject(annotation.task.boundingBox), color: [0, 0, 0], isVisible: true, id: largestId + 1, }); } @@ - volumeTracingMags: volumeTracingMags, - userBoundingBoxes: userBoundingBoxes || [], + volumeTracingMags, + userBoundingBoxes,Also applies to: 80-90, 98-104
15-113: Extract helpers to reduce cognitive loadSplit into small helpers: resolveInputsToAnnotationIds(inputs) and buildAnnotationBundle(annotationId). This improves readability and testability.
1-1: File placement nit: not a React hookThis module isn't a hook. Consider moving from hooks/ to utils/ or api/.
21-27: Robust URL parsing (handle query/fragment/trailing slash) and clearer var nameCurrent extraction breaks for URLs like ".../123?foo=bar" or with trailing "/". Decode and strip query/fragment. Also align naming with intent (annotationId).
Apply:
- if (taskOrAnnotationIdOrUrl.includes("/")) { - const lastSegment = taskOrAnnotationIdOrUrl.split("/").at(-1); - if (lastSegment) { - annotationIdsForTraining.push(lastSegment); - } - } else { + if (taskOrAnnotationIdOrUrl.includes("/")) { + // Extract annotation id from URL-like input (strip trailing slashes, query, fragment) + let lastPathSegment = ""; + try { + const u = new URL(taskOrAnnotationIdOrUrl); + lastPathSegment = u.pathname.replace(/\/+$/, "").split("/").at(-1) ?? ""; + } catch { + // Not an absolute URL; treat as path-like string + lastPathSegment = taskOrAnnotationIdOrUrl.replace(/\/+$/, "").split("/").at(-1) ?? ""; + } + const annotationId = lastPathSegment.split(/[?#]/, 1)[0]; + if (annotationId) { + annotationIdsForTraining.push(decodeURIComponent(annotationId)); + } else { + Toast.warning(`Could not extract annotation id from "${taskOrAnnotationIdOrUrl}"`); + } + } else {
🧹 Nitpick comments (5)
frontend/javascripts/viewer/view/ai_jobs/utils.ts (1)
325-343: Wrap getIntersectingMagList call in ai_training_data_selector.tsx in try/catchgetIntersectingMagList invokes getSegmentationLayerByHumanReadableName, which throws if the layer isn’t found; handling this prevents uncaught errors in the component.
frontend/javascripts/viewer/view/ai_jobs/hooks/fetch_annotation_infos.ts (4)
33-34: Avoid magic string for annotation stateReplace "Finished" with a shared enum/constant if available to prevent drift.
Do we have a central AnnotationState/TaskState enum in types? If yes, import and use it here.
48-50: Deduplicate annotation IDs before fetching to avoid duplicate requests/UI entriesUse a Set to dedupe.
- const newAnnotationsWithDatasets = await Promise.all( - annotationIdsForTraining.map(async (annotationId) => { + const uniqueAnnotationIdsForTraining = Array.from(new Set(annotationIdsForTraining)); + const newAnnotationsWithDatasets = await Promise.all( + uniqueAnnotationIdsForTraining.map(async (annotationId) => {
53-60: Use enum instead of string literal for layer type + parallelize dataset and tracing fetches
- Prefer AnnotationLayerEnum.Volume for consistency (you already use the enum for Skeleton).
- Fetch dataset and tracings concurrently to reduce latency.
- const dataset = await getDataset(annotation.datasetId); - - const volumeServerTracings: ServerVolumeTracing[] = await Promise.all( - annotation.annotationLayers - .filter((layer) => layer.typ === "Volume") - .map( - (layer) => - getTracingForAnnotationType(annotation, layer) as Promise<ServerVolumeTracing>, - ), - ); + const [dataset, volumeServerTracings] = await Promise.all([ + getDataset(annotation.datasetId), + Promise.all( + annotation.annotationLayers + .filter((layer) => layer.typ === AnnotationLayerEnum.Volume) + .map( + (layer) => + getTracingForAnnotationType(annotation, layer) as Promise<ServerVolumeTracing>, + ), + ), + ]);
19-20: De-dup unfinished tasks to avoid repeated names in the toastUse Set to collect, then spread on display.
- const unfinishedTasks: string[] = []; + const unfinishedTasks = new Set<string>(); @@ - unfinishedTasks.push(taskOrAnnotationIdOrUrl); + unfinishedTasks.add(taskOrAnnotationIdOrUrl); @@ - if (unfinishedTasks.length > 0) { - Toast.warning( - `The following tasks have no finished annotations: ${unfinishedTasks.join(", ")}`, - ); + if (unfinishedTasks.size > 0) { + Toast.warning( + `The following tasks have no finished annotations: ${[...unfinishedTasks].join(", ")}`, + ); }Also applies to: 37-38, 107-111
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
frontend/javascripts/types/api_types.ts(1 hunks)frontend/javascripts/viewer/view/action-bar/create_animation_modal.tsx(1 hunks)frontend/javascripts/viewer/view/action-bar/download_modal_view.tsx(1 hunks)frontend/javascripts/viewer/view/ai_jobs/components/bounding_box_selection_form_item.tsx(1 hunks)frontend/javascripts/viewer/view/ai_jobs/hooks/fetch_annotation_infos.ts(1 hunks)frontend/javascripts/viewer/view/ai_jobs/utils.ts(5 hunks)frontend/javascripts/viewer/view/left-border-tabs/layer_settings_tab.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- frontend/javascripts/viewer/view/action-bar/create_animation_modal.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- frontend/javascripts/types/api_types.ts
- frontend/javascripts/viewer/view/ai_jobs/components/bounding_box_selection_form_item.tsx
- frontend/javascripts/viewer/view/action-bar/download_modal_view.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
frontend/javascripts/viewer/view/ai_jobs/hooks/fetch_annotation_infos.ts (7)
frontend/javascripts/viewer/view/ai_jobs/utils.ts (1)
AnnotationInfoForAITrainingJob(141-147)frontend/javascripts/types/api_types.ts (2)
APIAnnotation(589-591)ServerVolumeTracing(974-1001)frontend/javascripts/admin/api/tasks.ts (1)
getAnnotationsForTask(29-34)frontend/javascripts/admin/rest_api.ts (3)
getUnversionedAnnotationInformation(618-629)getDataset(1032-1043)getTracingForAnnotationType(740-781)frontend/javascripts/viewer/model/reducers/volumetracing_reducer.ts (1)
serverVolumeToClientVolumeTracing(268-324)frontend/javascripts/viewer/model/reducers/reducer_helpers.ts (1)
convertUserBoundingBoxesFromServerToFrontend(68-85)frontend/javascripts/viewer/constants.ts (1)
Vector3(14-14)
frontend/javascripts/viewer/view/ai_jobs/utils.ts (3)
frontend/javascripts/viewer/view/ai_jobs/constants.ts (1)
MIN_BBOX_EXTENT(24-35)frontend/javascripts/types/api_types.ts (3)
APIDataLayer(114-114)APIAnnotation(589-591)APIDataset(243-246)frontend/javascripts/viewer/model/accessors/volumetracing_accessor.ts (1)
getSegmentationLayerByHumanReadableName(139-162)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build-smoketest-push
- GitHub Check: frontend-tests
- GitHub Check: backend-tests
🔇 Additional comments (5)
frontend/javascripts/viewer/view/left-border-tabs/layer_settings_tab.tsx (1)
127-127: Import path update verified
The file exists at the new location and no imports reference the oldai_job_modals/materialize_volume_annotation_modalpath.frontend/javascripts/viewer/view/ai_jobs/utils.ts (4)
17-28: LGTM! Previous review feedback addressed.The addition of
INFER_INSTANCEScase and the default error throw correctly address the previous review comments. The grouping withINFER_NEURONSandINFER_NUCLEIfollows the suggested pattern, andMIN_BBOX_EXTENTincludes the necessary entry forINFER_INSTANCES.
45-87: LGTM! Consistent INFER_INSTANCES support.The function signature correctly extends
jobTypeto includeINFER_INSTANCES, and the logic appropriately groups it withINFER_MITOCHONDRIAto select the finest magnification. This is consistent with the comment indicating these models always infer on the finest mag.
89-139: LGTM! Type signatures correctly updated.Both
isBBoxTooSmallandisDatasetOrBoundingBoxTooSmallhave theirsegmentationTypeparameters appropriately extended to includeINFER_INSTANCES. The underlying logic remains sound and will work correctly with the new job type.
307-318: LGTM! Well-structured form validator.The
colorLayerMustNotBeUint24Rulefollows antd's validator pattern correctly. The null safety check, validation logic, and error message are all appropriate. This will integrate cleanly with antd Form components.
…sis_settings.tsx Co-authored-by: Philipp Otto <[email protected]>
…nto new_ai_job_ui
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 13
♻️ Duplicate comments (12)
frontend/javascripts/viewer/view/action_bar_view.tsx (1)
298-334: Critical issue from previous review remains unfixed: onClick handlers on menu items are ignored in antd v5.The onClick handlers on individual menu items (lines 301, 308, 315) are ignored by antd v5's Dropdown component. The AI Analysis dropdown buttons will not work. This was flagged as a critical issue in the previous review and the fix was not applied.
Apply the previously suggested fix by moving onClick logic to
menu.onClick:const isSuperUser = this.props.activeUser?.isSuperUser === true; const menuItems = [ - { - key: "open_ai_inference_button", - onClick: () => Store.dispatch(setAIJobDrawerStateAction("open_ai_inference")), - label: "Run AI model", - }, - ...(isSuperUser - ? [ - { - key: "open_ai_training_button", - onClick: () => Store.dispatch(setAIJobDrawerStateAction("open_ai_training")), - label: "Train new AI model", - }, - ] - : []), - { - key: "open_ai_alignment_button", - onClick: () => Store.dispatch(setAIJobDrawerStateAction("open_ai_alignment")), - label: "Run AI Alignment", - }, + { key: "open_ai_inference_button", label: "Run AI model" }, + ...(isSuperUser ? [{ key: "open_ai_training_button", label: "Train new AI model" }] : []), + { key: "open_ai_alignment_button", label: "Run AI Alignment" }, ]; return ( <div> <Dropdown key="ai-job-dropdown" menu={{ items: menuItems, + onClick: ({ key }) => { + switch (key) { + case "open_ai_inference_button": + Store.dispatch(setAIJobDrawerStateAction("open_ai_inference")); + break; + case "open_ai_training_button": + Store.dispatch(setAIJobDrawerStateAction("open_ai_training")); + break; + case "open_ai_alignment_button": + Store.dispatch(setAIJobDrawerStateAction("open_ai_alignment")); + break; + } + }, }} disabled={disabled} > <Button disabled={disabled} icon={<i className="fas fa-magic" />} title={tooltipText}> AI Analysis </Button> </Dropdown> </div> );Based on learnings
frontend/javascripts/viewer/view/ai_jobs/alignment/ai_alignment_job_context.tsx (3)
41-44: Add null checks for colorLayer to prevent runtime errors.If a dataset has no color layers,
colorLayers[0]will beundefined, causinggetBoundingBoxesForLayers([colorLayer])[0]to fail andcolorLayer.name(line 61) to throw. This issue was previously flagged but remains unresolved.Apply this diff to add guards:
const dataset = useWkSelector((state) => state.dataset); const annotationId = useWkSelector((state) => state.annotation.annotationId); const colorLayers = getColorLayers(dataset); - const colorLayer = colorLayers[0]; + const colorLayer = colorLayers[0] ?? null; - const selectedBoundingBox = getBoundingBoxesForLayers([colorLayer])[0]; + const selectedBoundingBox = colorLayer ? getBoundingBoxesForLayers([colorLayer])[0] : null;
52-55: Update validation to include colorLayer check.The validation doesn't check whether
colorLayerexists, buthandleStartAnalysisrequires it. This will allow the Start button to be enabled even when the job will fail.Apply this diff:
const areParametersValid = useMemo( - () => Boolean(selectedTask && selectedJobType && newDatasetName), - [selectedTask, selectedJobType, newDatasetName], + () => Boolean(selectedTask && selectedJobType && newDatasetName && colorLayer), + [selectedTask, selectedJobType, newDatasetName, colorLayer], );
57-71: Add early return guard for invalid parameters.The handler should verify parameters before attempting the API call. Without this guard, the function will crash on
colorLayer.namewhencolorLayerisnull.Apply this diff:
const handleStartAnalysis = useCallback(async () => { + if (!areParametersValid || !colorLayer) { + Toast.error("Please ensure all required parameters are set and a color layer exists."); + return; + } try { await startAlignSectionsJob( dataset.id, - colorLayer.name, + colorLayer!.name, newDatasetName, shouldUseManualMatches ? annotationId : undefined, ); Toast.success("Alignment started successfully!"); dispatch(setAIJobDrawerStateAction("invisible")); } catch (error) { console.error(error); Toast.error("Failed to start alignment."); } - }, [dataset.id, dispatch, colorLayer.name, newDatasetName, annotationId, shouldUseManualMatches]); + }, [dataset.id, dispatch, colorLayer, newDatasetName, annotationId, shouldUseManualMatches, areParametersValid]);frontend/javascripts/viewer/view/ai_jobs/run_ai_model/ai_analysis_settings.tsx (3)
149-158: Type‑narrow before mag/size validation.Guard to inference job types to avoid invalid calls.
- if (value && selectedLayer && selectedJobType) { + if ( + value && + selectedLayer && + (selectedJobType === APIJobType.INFER_MITOCHONDRIA || + selectedJobType === APIJobType.INFER_NEURONS || + selectedJobType === APIJobType.INFER_NUCLEI || + selectedJobType === APIJobType.INFER_INSTANCES) + ) {
61-65: Fix layer change handler: Select returns a string name.- if ("selectedLayer" in changedValues) { - setSelectedLayer( - colorLayers.find((l) => l.name === changedValues.selectedLayer.name) as APIDataLayer, - ); - } + if ("selectedLayer" in changedValues) { + const name = changedValues.selectedLayer as string; + setSelectedLayer(colorLayers.find((l) => l.name === name) as APIDataLayer); + }
127-139: Make uint24 validation effective for Select.The field stores a layer name (string). Validate against the resolved selectedLayer.
<Form.Item name="selectedLayer" label="Image Data Layer" rules={[ { required: true, message: "Please select an image data layer" }, - colorLayerMustNotBeUint24Rule, + { + validator: () => { + if (selectedLayer && (selectedLayer as any).elementClass === "uint24") { + return Promise.reject( + new Error( + "The selected layer of type uint24 is not supported. Please select a different one.", + ), + ); + } + return Promise.resolve(); + }, + }, ]} >docs/automation/ai_training.md (1)
26-28: Update “dialog” to “drawer” and reference the tab.The new UX is a drawer with a “Model Training” tab; adjust copy accordingly.
-## Configuring the Training -To start a training, click on the `AI Analysis` button in the toolbar and select `Train AI model` from the dropdown menu. -This will open a dialog where you can configure and start your training job. +## Configuring the Training +To start training, click the `AI Analysis` button in the toolbar and select `Train AI model`. +This opens a drawer with a `Model Training` tab where you can configure and start your training job.frontend/javascripts/viewer/view/ai_jobs/run_ai_model/ai_model_selector.tsx (1)
54-65: Fix category→jobType mapping; include NUCLEI and MITOCHONDRIA.EM_NUCLEI maps to INFER_NUCLEI (not INSTANCES). Also add EM_MITOCHONDRIA → INFER_MITOCHONDRIA to avoid wrong pipelines/costs and errors on selection.
-const mapCategoryToJobType = ( - category: APIAiModelCategory, -): APIJobType.INFER_NEURONS | APIJobType.INFER_INSTANCES => { +const mapCategoryToJobType = ( + category: APIAiModelCategory, +): AllowedJobType => { switch (category) { case APIAiModelCategory.EM_NEURONS: return APIJobType.INFER_NEURONS; case APIAiModelCategory.EM_NUCLEI: - return APIJobType.INFER_INSTANCES; + return APIJobType.INFER_NUCLEI; + case APIAiModelCategory.EM_MITOCHONDRIA: + return APIJobType.INFER_MITOCHONDRIA; default: throw new Error(`Unsupported category: ${category}`); } };Also update types where used:
Type alias (near Lines 13-24):
type AllowedJobType = | APIJobType.INFER_NEURONS | APIJobType.INFER_NUCLEI | APIJobType.INFER_MITOCHONDRIA | APIJobType.INFER_INSTANCES; type PretrainedModel = { /* ... */ jobType: AllowedJobType; /* ... */ };onSelectModel (Lines 84-88):
let jobType: AllowedJobType;frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_job_context.tsx (3)
11-11: Remove lodash/every import; use native checks.Drop the dependency and avoid truthiness pitfalls.
-import every from "lodash/every";
78-82: Avoid non-null assertion in queryFn.Read id from queryKey to drop the unsafe "!".
- const { data: initialFullAnnotation } = useQuery({ - queryKey: ["initialAnnotation", currentAnnotation.annotationId], - queryFn: () => getUnversionedAnnotationInformation(currentAnnotation.annotationId!), - enabled: !!currentAnnotation.annotationId, - }); + const { data: initialFullAnnotation } = useQuery({ + queryKey: ["initialAnnotation", currentAnnotation.annotationId], + queryFn: ({ queryKey }) => getUnversionedAnnotationInformation(queryKey[1] as string), + enabled: Boolean(currentAnnotation.annotationId), + });
124-131: Replace fragile validity check; accept enum 0 and validate Vector3.Current check can misclassify valid values and blocks TS narrowing. Use explicit checks and a type guard.
- const areParametersValid = useMemo(() => { - const areSelectionsValid = every( - selectedAnnotations, - (s) => s.imageDataLayer && s.groundTruthLayer && s.magnification, - ); - - return every([modelName, selectedJobType, areSelectionsValid, selectedAnnotations.length > 0]); - }, [modelName, selectedJobType, selectedAnnotations]); + const areParametersValid = useMemo(() => { + const hasAtLeastOne = selectedAnnotations.length > 0; + const areSelectionsValid = selectedAnnotations.every(isCompleteSelection); + return modelName.trim().length > 0 && selectedJobType != null && hasAtLeastOne && areSelectionsValid; + }, [modelName, selectedJobType, selectedAnnotations]);Add near the interfaces:
type CompleteSelection = AiTrainingAnnotationSelection & Required<Pick<AiTrainingAnnotationSelection, "imageDataLayer" | "groundTruthLayer" | "magnification">>; const isVector3 = (v: unknown): v is Vector3 => Array.isArray(v) && v.length === 3 && v.every((n) => typeof n === "number" && Number.isFinite(n)); const isCompleteSelection = (s: AiTrainingAnnotationSelection): s is CompleteSelection => typeof s.imageDataLayer === "string" && s.imageDataLayer.length > 0 && typeof s.groundTruthLayer === "string" && s.groundTruthLayer.length > 0 && isVector3(s.magnification);Based on learnings
🧹 Nitpick comments (4)
frontend/javascripts/viewer/view/ai_jobs/run_ai_model/ai_model_selector.tsx (1)
224-228: Label custom model categories precisely.Show NUCLEI and MITOCHONDRIA tags instead of generic INSTANCES.
- <Tag> - {(item as AiModel).category === APIAiModelCategory.EM_NEURONS - ? "NEURONS" - : "INSTANCES"} - </Tag> + <Tag> + {(item as AiModel).category === APIAiModelCategory.EM_NEURONS + ? "NEURONS" + : (item as AiModel).category === APIAiModelCategory.EM_NUCLEI + ? "NUCLEI" + : (item as AiModel).category === APIAiModelCategory.EM_MITOCHONDRIA + ? "MITOCHONDRIA" + : "INSTANCES"} + </Tag>frontend/javascripts/viewer/view/ai_jobs/run_ai_model/ai_analysis_settings.tsx (1)
191-197: Avoid defaultValue on a controlled Form field.Remove defaultValue to prevent controlled/uncontrolled mismatch.
- <InputNumber + <InputNumber min={0.1} suffix="nm" style={{ width: "100%" }} - defaultValue={1000} disabled={!isInstanceModel} />frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_job_context.tsx (2)
148-152: Typo: rename commonJobArgmuments → commonJobArguments.Small clarity tweak.
- const commonJobArgmuments = { + const commonJobArguments = { trainingAnnotations: trainingAnnotations, name: modelName, comment: comments, }; @@ - ...commonJobArgmuments, + ...commonJobArguments, @@ - ...commonJobArgmuments, + ...commonJobArguments,Also applies to: 159-159, 165-165
183-188: Expose setters with the declared narrower signatures.Wrap useState dispatchers to match AiTrainingJobContextType and avoid passing the broader SetStateAction signature.
const value = { selectedJobType, selectedTask, - setSelectedJobType, - setSelectedTask, + setSelectedJobType: setSelectedJobTypeValue, + setSelectedTask: setSelectedTaskValue, handleStartAnalysis,Add near the other callbacks:
const setSelectedJobTypeValue = useCallback((jobType: APIJobType) => setSelectedJobType(jobType), []); const setSelectedTaskValue = useCallback((task: AiTrainingTask) => setSelectedTask(task), []);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
docs/automation/ai_training.md(1 hunks)frontend/javascripts/components/stacked_bar_chart.tsx(2 hunks)frontend/javascripts/viewer/view/action_bar_view.tsx(4 hunks)frontend/javascripts/viewer/view/ai_jobs/alignment/ai_alignment_job_context.tsx(1 hunks)frontend/javascripts/viewer/view/ai_jobs/alignment/ai_alignment_model_selector.tsx(1 hunks)frontend/javascripts/viewer/view/ai_jobs/alignment/ai_alignment_settings.tsx(1 hunks)frontend/javascripts/viewer/view/ai_jobs/hooks/fetch_annotation_infos.ts(1 hunks)frontend/javascripts/viewer/view/ai_jobs/run_ai_model/ai_analysis_settings.tsx(1 hunks)frontend/javascripts/viewer/view/ai_jobs/run_ai_model/ai_model_selector.tsx(1 hunks)frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_data_selector.tsx(1 hunks)frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_job_context.tsx(1 hunks)frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_model_selector.tsx(1 hunks)frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_settings.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/javascripts/viewer/view/ai_jobs/hooks/fetch_annotation_infos.ts
- frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_settings.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-27T11:32:54.033Z
Learnt from: knollengewaechs
PR: scalableminds/webknossos#8796
File: frontend/javascripts/viewer/view/action-bar/ai_job_modals/forms/neuron_segmentation_form.tsx:68-77
Timestamp: 2025-08-27T11:32:54.033Z
Learning: In the webknossos frontend AI job forms, `datasetConfiguration.layers[colorLayer.name]` is assumed to always be non-null when accessing layer properties like `isInverted`. The selected color layer is guaranteed to exist in the dataset configuration.
Applied to files:
frontend/javascripts/viewer/view/ai_jobs/alignment/ai_alignment_job_context.tsx
🧬 Code graph analysis (10)
frontend/javascripts/viewer/view/ai_jobs/run_ai_model/ai_model_selector.tsx (5)
frontend/javascripts/viewer/view/ai_jobs/run_ai_model/ai_image_segmentation_job_context.tsx (1)
useRunAiModelJobContext(276-282)frontend/javascripts/libs/react_helpers.tsx (1)
useGuardedFetch(54-90)frontend/javascripts/admin/api/jobs.ts (1)
getAiModels(443-449)frontend/javascripts/types/api_types.ts (1)
AiModel(833-844)frontend/javascripts/theme.tsx (1)
ColorWKBlue(12-12)
frontend/javascripts/viewer/view/ai_jobs/alignment/ai_alignment_model_selector.tsx (2)
frontend/javascripts/viewer/view/ai_jobs/alignment/ai_alignment_job_context.tsx (1)
useAlignmentJobContext(90-96)frontend/javascripts/theme.tsx (1)
ColorWKBlue(12-12)
frontend/javascripts/viewer/view/ai_jobs/alignment/ai_alignment_settings.tsx (3)
frontend/javascripts/viewer/view/ai_jobs/alignment/ai_alignment_job_context.tsx (1)
useAlignmentJobContext(90-96)frontend/javascripts/theme.tsx (1)
ColorWKBlue(12-12)frontend/javascripts/viewer/view/ai_jobs/components/should_use_trees_form_item.tsx (1)
ShouldUseManualMatchesFormItem(7-61)
frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_data_selector.tsx (6)
frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_job_context.tsx (2)
AiTrainingAnnotationSelection(22-29)useAiTrainingJobContext(204-210)frontend/javascripts/viewer/view/ai_jobs/utils.ts (2)
getIntersectingMagList(325-343)colorLayerMustNotBeUint24Rule(307-318)frontend/javascripts/libs/utils.ts (1)
computeVolumeFromBoundingBox(314-317)frontend/javascripts/libs/format_utils.ts (1)
formatVoxels(515-538)frontend/javascripts/theme.tsx (1)
ColorWKBlue(12-12)frontend/javascripts/viewer/view/ai_jobs/train_ai_model/annotations_csv_input.tsx (1)
AnnotationsCsvInput(8-79)
frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_job_context.tsx (9)
frontend/javascripts/types/api_types.ts (2)
APIAnnotation(589-591)APIDataset(243-246)frontend/javascripts/viewer/constants.ts (1)
Vector3(14-14)frontend/javascripts/viewer/store.ts (1)
UserBoundingBox(108-110)frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_model_selector.tsx (1)
AiTrainingTask(11-18)frontend/javascripts/libs/react_hooks.ts (1)
useWkSelector(292-294)frontend/javascripts/viewer/model/accessors/tracing_accessor.ts (1)
getUserBoundingBoxesFromState(98-101)frontend/javascripts/admin/rest_api.ts (1)
getUnversionedAnnotationInformation(618-629)frontend/javascripts/admin/api/jobs.ts (3)
AiModelTrainingAnnotationSpecification(371-376)runInstanceModelTraining(402-407)runNeuronTraining(386-391)frontend/javascripts/viewer/model/actions/ui_actions.ts (1)
setAIJobDrawerStateAction(136-140)
frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_model_selector.tsx (2)
frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_job_context.tsx (1)
useAiTrainingJobContext(204-210)frontend/javascripts/theme.tsx (1)
ColorWKBlue(12-12)
frontend/javascripts/viewer/view/action_bar_view.tsx (1)
frontend/javascripts/viewer/model/actions/ui_actions.ts (1)
setAIJobDrawerStateAction(136-140)
frontend/javascripts/components/stacked_bar_chart.tsx (1)
frontend/javascripts/viewer/model/helpers/iterator_utils.ts (1)
sum(29-31)
frontend/javascripts/viewer/view/ai_jobs/run_ai_model/ai_analysis_settings.tsx (10)
frontend/javascripts/viewer/view/ai_jobs/run_ai_model/ai_image_segmentation_job_context.tsx (1)
useRunAiModelJobContext(276-282)frontend/javascripts/libs/react_hooks.ts (1)
useWkSelector(292-294)frontend/javascripts/types/api_types.ts (1)
APIDataLayer(114-114)frontend/javascripts/viewer/view/ai_jobs/components/collapsible_split_merger_evaluation_settings.tsx (2)
SplitMergerEvaluationSettings(4-9)CollapsibleSplitMergerEvaluationSettings(11-90)frontend/javascripts/theme.tsx (1)
ColorWKBlue(12-12)frontend/javascripts/admin/dataset/dataset_components.tsx (1)
getDatasetNameRules(68-85)frontend/javascripts/viewer/view/ai_jobs/utils.ts (3)
colorLayerMustNotBeUint24Rule(307-318)getBestFittingMagComparedToTrainingDS(45-87)isDatasetOrBoundingBoxTooSmall(116-139)frontend/javascripts/viewer/store.ts (1)
UserBoundingBox(108-110)frontend/javascripts/libs/utils.ts (1)
computeArrayFromBoundingBox(299-308)frontend/javascripts/viewer/view/ai_jobs/bounding_box_selector.tsx (1)
BoundingBoxSelector(15-38)
frontend/javascripts/viewer/view/ai_jobs/alignment/ai_alignment_job_context.tsx (7)
frontend/javascripts/viewer/view/ai_jobs/alignment/ai_alignment_model_selector.tsx (1)
AlignmentTask(11-18)frontend/javascripts/viewer/store.ts (1)
UserBoundingBox(108-110)frontend/javascripts/libs/react_hooks.ts (1)
useWkSelector(292-294)frontend/javascripts/viewer/view/ai_jobs/utils.ts (1)
getBoundingBoxesForLayers(30-40)frontend/javascripts/admin/api/jobs.ts (1)
startAlignSectionsJob(344-363)frontend/javascripts/viewer/model/actions/ui_actions.ts (1)
setAIJobDrawerStateAction(136-140)frontend/javascripts/libs/toast.tsx (1)
error(211-218)
🪛 GitHub Check: frontend-tests
frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_job_context.tsx
[failure] 139-139:
Type '{ annotationId: string; colorLayerName: string | undefined; segmentationLayerName: string | undefined; mag: Vector3 | undefined; }[]' is not assignable to type 'AiModelTrainingAnnotationSpecification[]'.
🪛 LanguageTool
docs/automation/ai_training.md
[style] ~7-~7: For conciseness, consider replacing this expression with an adverb.
Context: ...webknossos.org](https://webknossos.org) at the moment. If you want to set up on-premise ...
(AT_THE_MOMENT)
[grammar] ~46-~46: There might be a mistake here.
Context: ... Give your new model a descriptive name. * Comments: Add any comments or notes ab...
(QB_NEW_EN)
[grammar] ~58-~58: Ensure spelling is correct
Context: ... the training is finished, you can find and overview of all your trained model from...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~58-~58: There might be a mistake here.
Context: ...all your trained model from the Admin >AI Models page in the navbar. Please refe...
(QB_NEW_EN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-smoketest-push
- GitHub Check: backend-tests
🔇 Additional comments (2)
frontend/javascripts/components/stacked_bar_chart.tsx (1)
1-1: LGTM! Modular import improves bundle size.The switch from the default lodash import to a specific function import (
lodash/sum) is a recommended best practice that enables tree-shaking and reduces bundle size. The usage is updated correctly on line 20.Based on learnings
Also applies to: 20-20
frontend/javascripts/viewer/view/ai_jobs/alignment/ai_alignment_settings.tsx (1)
10-55: LGTM!The alignment settings component is well-structured with proper context integration, controlled form state, and appropriate validation rules. The two-column layout cleanly separates dataset naming from manual match configuration.
frontend/javascripts/viewer/view/ai_jobs/alignment/ai_alignment_model_selector.tsx
Outdated
Show resolved
Hide resolved
frontend/javascripts/viewer/view/ai_jobs/run_ai_model/ai_analysis_settings.tsx
Outdated
Show resolved
Hide resolved
| label="Magnification" | ||
| required | ||
| rules={[{ required: true, message: "Please select a magnification" }]} | ||
| validateStatus={magnificationValidationError ? "error" : undefined} | ||
| help={magnificationValidationError} | ||
| > |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add name for Magnification so required rule can trigger.
- <Form.Item
+ <Form.Item
+ name={["annotations", annotationId, "magnification"]}
label="Magnification"
required
rules={[{ required: true, message: "Please select a magnification" }]}
validateStatus={magnificationValidationError ? "error" : undefined}
help={magnificationValidationError}
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| label="Magnification" | |
| required | |
| rules={[{ required: true, message: "Please select a magnification" }]} | |
| validateStatus={magnificationValidationError ? "error" : undefined} | |
| help={magnificationValidationError} | |
| > | |
| <Form.Item | |
| name={["annotations", annotationId, "magnification"]} | |
| label="Magnification" | |
| required | |
| rules={[{ required: true, message: "Please select a magnification" }]} | |
| validateStatus={magnificationValidationError ? "error" : undefined} | |
| help={magnificationValidationError} | |
| > |
🤖 Prompt for AI Agents
In
frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_data_selector.tsx
around lines 184-189, the Form.Item for "Magnification" is missing the name prop
so the required validation rule won't trigger; add a name="magnification" (or
whatever form field key is used elsewhere for magnification) to the Form.Item so
the Ant Design form can wire up and enforce the required rule, keeping existing
rules, validateStatus and help as-is.
frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_job_context.tsx
Outdated
Show resolved
Hide resolved
| const trainingAnnotations: AiModelTrainingAnnotationSpecification[] = selectedAnnotations.map( | ||
| (selection) => ({ | ||
| annotationId: selection.annotation.id, | ||
| colorLayerName: selection.imageDataLayer, | ||
| segmentationLayerName: selection.groundTruthLayer, | ||
| mag: selection.magnification, | ||
| }), | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix TS error: map only complete selections to satisfy AiModelTrainingAnnotationSpecification.
Use the type guard to ensure all fields are defined.
- const trainingAnnotations: AiModelTrainingAnnotationSpecification[] = selectedAnnotations.map(
- (selection) => ({
- annotationId: selection.annotation.id,
- colorLayerName: selection.imageDataLayer,
- segmentationLayerName: selection.groundTruthLayer,
- mag: selection.magnification,
- }),
- );
+ const completeSelections = selectedAnnotations.filter(isCompleteSelection);
+ const trainingAnnotations: AiModelTrainingAnnotationSpecification[] = completeSelections.map(
+ (selection) => ({
+ annotationId: selection.annotation.id,
+ colorLayerName: selection.imageDataLayer,
+ segmentationLayerName: selection.groundTruthLayer,
+ mag: selection.magnification,
+ }),
+ );Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 GitHub Check: frontend-tests
[failure] 139-139:
Type '{ annotationId: string; colorLayerName: string | undefined; segmentationLayerName: string | undefined; mag: Vector3 | undefined; }[]' is not assignable to type 'AiModelTrainingAnnotationSpecification[]'.
🤖 Prompt for AI Agents
In
frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_job_context.tsx
around lines 139-146, the current map over selectedAnnotations can produce
undefined fields and fails the AiModelTrainingAnnotationSpecification type; add
a type-guarded filter step (or inline filter using a user-defined type guard)
that ensures selection.annotation and selection.annotation.id,
selection.imageDataLayer, selection.groundTruthLayer, and
selection.magnification are all defined, then map the filtered array to produce
AiModelTrainingAnnotationSpecification objects—this guarantees the compiler that
all required fields are present and keeps the resulting array typed correctly.
frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_model_selector.tsx
Outdated
Show resolved
Hide resolved
frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_model_selector.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
frontend/javascripts/viewer/view/ai_jobs/hooks/fetch_annotation_infos.ts (1)
20-21: Clarify support for "task URLs" in documentation.The function comment claims to parse "task IDs, annotation IDs, or annotation URLs," but past review discussions indicate that task URLs don't exist in the system. The current implementation only handles annotation URLs (lines 27-31) by extracting the annotation ID from the path. Consider updating the JSDoc and parameter name to accurately reflect this limitation.
🧹 Nitpick comments (2)
frontend/javascripts/viewer/view/ai_jobs/hooks/fetch_annotation_infos.ts (2)
27-31: Strengthen URL parsing to handle edge cases.The current split-by-slash approach may fail if the URL contains trailing slashes, query parameters, or fragment identifiers. A URL ending in
/#or including?param=valuecould produce an incorrect or empty annotation ID.Consider using the URL API for more robust parsing:
if (taskOrAnnotationIdOrUrl.includes("/")) { - const annotationId = taskOrAnnotationIdOrUrl.split("/").at(-1); + try { + const url = new URL(taskOrAnnotationIdOrUrl, window.location.origin); + const annotationId = url.pathname.split("/").filter(Boolean).at(-1); + } catch { + // If URL parsing fails, fall back to simple split + const annotationId = taskOrAnnotationIdOrUrl.split("/").filter(Boolean).at(-1); + } if (annotationId) { annotationIdsForTraining.push(annotationId); }
86-120: LGTM! Consider clarifying the comment about the original bug.The bounding box extraction logic is solid: it correctly handles empty volume tracings via optional chaining, falls back to skeleton layers when needed, throws an appropriate error if neither exists, and properly appends the task bounding box.
However, the comment on line 93 references "the original code had a bug" which might confuse future readers since the bug is no longer present.
Consider updating the comment to be more forward-looking:
- // The original code had a bug here (!userBoundingBoxes) which was always false for an empty array. + // Check if the array is empty (not just falsy) const skeletonLayer = annotation.annotationLayers.find(
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/javascripts/viewer/view/ai_jobs/hooks/fetch_annotation_infos.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/javascripts/viewer/view/ai_jobs/hooks/fetch_annotation_infos.ts (8)
frontend/javascripts/admin/api/tasks.ts (1)
getAnnotationsForTask(29-34)frontend/javascripts/types/api_types.ts (2)
APIAnnotation(589-591)ServerVolumeTracing(974-1001)frontend/javascripts/admin/rest_api.ts (3)
getTracingForAnnotationType(740-781)getUnversionedAnnotationInformation(618-629)getDataset(1032-1043)frontend/javascripts/viewer/constants.ts (1)
Vector3(14-14)frontend/javascripts/viewer/store.ts (1)
VolumeTracing(192-213)frontend/javascripts/viewer/model/reducers/reducer_helpers.ts (1)
convertUserBoundingBoxesFromServerToFrontend(68-85)frontend/javascripts/viewer/view/ai_jobs/utils.ts (1)
AnnotationInfoForAITrainingJob(141-147)frontend/javascripts/viewer/model/reducers/volumetracing_reducer.ts (1)
serverVolumeToClientVolumeTracing(268-324)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-smoketest-push
- GitHub Check: backend-tests
🔇 Additional comments (4)
frontend/javascripts/viewer/view/ai_jobs/hooks/fetch_annotation_infos.ts (4)
55-67: LGTM!The function correctly filters volume layers and fetches their tracings concurrently.
69-79: LGTM!The magnification extraction correctly handles both legacy annotations (without
mags) and modern ones, defaulting to[1,1,1]as expected.
122-150: LGTM!The function properly orchestrates fetching and transforming all required annotation data with appropriate parallelization.
158-181: LGTM!The main exported function orchestrates the complete workflow effectively: resolving IDs, parallel fetching, user warnings for unfinished tasks, and comprehensive error handling with console logging and toast notifications. Returning an empty array on error is reasonable since
Promise.allmeans partial success isn't possible.
frontend/javascripts/viewer/view/ai_jobs/hooks/fetch_annotation_infos.ts
Show resolved
Hide resolved
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…sis_settings.tsx Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…nto new_ai_job_ui
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (8)
frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_job_context.tsx (2)
11-11: Replace lodasheverywith explicit validation.The lodash
everycall is fragile because it relies on truthiness, which can incorrectly reject valid falsy values (e.g.,0for an enum). Additionally,every([])returnstrue, though you mitigate this separately. Use explicit boolean checks for clarity and correctness.Apply this diff:
-import every from "lodash/every"; import type React from "react";const areParametersValid = useMemo(() => { - const areSelectionsValid = every( - selectedAnnotations, - (s) => s.imageDataLayer && s.groundTruthLayer && s.magnification, - ); - - return every([modelName, selectedJobType, areSelectionsValid, selectedAnnotations.length > 0]); + const areSelectionsValid = + selectedAnnotations.length > 0 && + selectedAnnotations.every( + (s) => + Boolean(s.imageDataLayer) && + Boolean(s.groundTruthLayer) && + Array.isArray(s.magnification) && + s.magnification.length === 3, + ); + return modelName.trim().length > 0 && selectedJobType != null && areSelectionsValid; }, [modelName, selectedJobType, selectedAnnotations]);Also applies to: 124-131
139-146: Filter for complete selections before mapping.TypeScript fails because optional fields can be
undefined, butAiModelTrainingAnnotationSpecificationrequires all fields to be defined. Filter the selections first using a type guard.Add a type guard before the mapping:
+ // Type guard to ensure all required fields are present + const isCompleteSelection = ( + selection: AiTrainingAnnotationSelection, + ): selection is Required<AiTrainingAnnotationSelection> => + Boolean(selection.imageDataLayer) && + Boolean(selection.groundTruthLayer) && + Boolean(selection.magnification); + + const completeSelections = selectedAnnotations.filter(isCompleteSelection); + if (completeSelections.length !== selectedAnnotations.length) { + Toast.error("Some annotations are missing required fields."); + return; + } + - const trainingAnnotations: AiModelTrainingAnnotationSpecification[] = selectedAnnotations.map( + const trainingAnnotations: AiModelTrainingAnnotationSpecification[] = completeSelections.map( (selection) => ({frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_model_selector.tsx (1)
24-24: Fix grammar: add missing preposition.The phrase "Optimized dense neuronal tissue" is missing "for" before "dense."
Apply this diff:
- "Train a new AI model for EM neuron segmentation based on the annotations in this dataset. Optimized dense neuronal tissue from SEM, FIB-SEM, SBEM, Multi-SEM microscopes.", + "Train a new AI model for EM neuron segmentation based on the annotations in this dataset. Optimized for dense neuronal tissue from SEM, FIB-SEM, SBEM, Multi-SEM microscopes.",frontend/javascripts/viewer/view/ai_jobs/run_ai_model/ai_analysis_settings.tsx (3)
61-65: Critical: Fix the selectedLayer change handler.The
Selectcomponent returns the layer name as a string (becauseoptionsmapsvalue: l.name), but line 63 incorrectly accesseschangedValues.selectedLayer.name. This will causeselectedLayerto remainundefined.Apply this diff:
if ("selectedLayer" in changedValues) { setSelectedLayer( - colorLayers.find((l) => l.name === changedValues.selectedLayer.name) as APIDataLayer, + colorLayers.find((l) => l.name === changedValues.selectedLayer) as APIDataLayer, ); }
127-139: Critical: uint24 validation is ineffective.The field stores a string (layer name), but
colorLayerMustNotBeUint24Ruleexpects anAPIDataLayerobject. The validator will never reject uint24 layers becausevalue.elementClassis undefined on strings.Apply this diff to validate against the resolved layer:
<Form.Item name="selectedLayer" label="Image Data Layer" rules={[ { required: true, message: "Please select an image data layer" }, - colorLayerMustNotBeUint24Rule, + { + validator: () => { + if (selectedLayer && selectedLayer.elementClass === "uint24") { + return Promise.reject( + new Error( + "The selected layer of type uint24 is not supported. Please select a different one.", + ), + ); + } + return Promise.resolve(); + }, + }, ]} >
149-169: Critical: Add type guard before calling inference-only functions.Both
getBestFittingMagComparedToTrainingDSandisDatasetOrBoundingBoxTooSmallaccept only inference job types (INFER_MITOCHONDRIA | INFER_NEURONS | INFER_NUCLEI | INFER_INSTANCES). Calling them with a training or alignment job type will cause type errors.Add a type guard at line 150:
{ validator: (_, value: UserBoundingBox) => { if (value && selectedLayer && selectedJobType) { + // Only validate for inference jobs + const inferenceJobTypes = ["infer_mitochondria", "infer_neurons", "infer_nuclei", "infer_instances"]; + if (!inferenceJobTypes.includes(selectedJobType)) { + return Promise.resolve(); + } + const boundingBox = computeArrayFromBoundingBox(value.boundingBox); const mag = getBestFittingMagComparedToTrainingDS( selectedLayer, dataset.dataSource.scale, selectedJobType, );Alternatively, if
selectedJobTypehas a discriminated union type, use a type predicate or check a discriminant property.docs/automation/ai_training.md (2)
26-27: Update UI copy: modal → drawer, include tab name.The UI now uses a drawer with a “Model Training” tab, not a dialog.
-To start a training, click on the `AI Analysis` button in the toolbar and select `Train AI model` from the dropdown menu. -This will open a dialog where you can configure and start your training job. +To start training, click the `AI Analysis` button in the toolbar and choose `Train AI model`. +This opens a drawer with a `Model Training` tab where you can configure and start your training job.
7-8: Tighten style and fix “on‑premise” → “on‑premises”.- AI Model Training is only available on [webknossos.org](https://webknossos.org) at the moment. - If you want to set up on-premise automated analysis at your institute/workplace, then [please contact sales](mailto:[email protected]). + AI Model Training is currently available on [webknossos.org](https://webknossos.org). + If you want to set up on‑premises automated analysis at your institute/workplace, [please contact sales](mailto:[email protected]).
🧹 Nitpick comments (3)
frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_job_context.tsx (1)
80-80: Avoid non-null assertion; use type guard.The
!is safe here because the query isenabledonly whenannotationIdis truthy, but non-null assertions can mask refactoring risks and are a code smell.Apply this diff to use optional chaining instead:
- queryFn: () => getUnversionedAnnotationInformation(currentAnnotation.annotationId!), + queryFn: () => + currentAnnotation.annotationId + ? getUnversionedAnnotationInformation(currentAnnotation.annotationId) + : Promise.reject(new Error("No annotation ID")), enabled: !!currentAnnotation.annotationId,docs/automation/ai_training.md (2)
23-23: Grammar/clarity: labeling paragraph.-3. **Label segments:** Within your bounding boxes, label the segmentation of your structures of interest. Use the volume annotation tool to manually annotate structures. This will be your ground truth data. For neurons, we recommend to densely label each structure with a unique ID. For instances segmentations you only need to label the structures you want to train on, e.g. nuclei, mitochondria, soma, vesicles, etc. +3. **Label segments:** Within your bounding boxes, label the structures of interest using the volume annotation tool. This will be your ground truth data. For neurons, we recommend densely labeling each structure with a unique ID. For instance segmentations, you only need to label the structures you want to train on (e.g., nuclei, mitochondria, soma, vesicles).
48-48: Units/examples formatting.-* **Max Distance (nm):** (Only for EM Instances Model) The maximum cross-section length ("diameter") for each identified object in nanometers e.g. Nuclei: 1000nm, Vesicles: 80nm. +* **Max Distance (nm):** (Only for EM Instances Model) The maximum cross‑section length (“diameter”) for each identified object in nanometers (e.g., nuclei: 1,000 nm; vesicles: 80 nm).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
docs/automation/ai_training.md(1 hunks)frontend/javascripts/viewer/view/ai_jobs/alignment/ai_alignment_model_selector.tsx(1 hunks)frontend/javascripts/viewer/view/ai_jobs/run_ai_model/ai_analysis_settings.tsx(1 hunks)frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_job_context.tsx(1 hunks)frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_model_selector.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/javascripts/viewer/view/ai_jobs/alignment/ai_alignment_model_selector.tsx
🧰 Additional context used
🧬 Code graph analysis (3)
frontend/javascripts/viewer/view/ai_jobs/run_ai_model/ai_analysis_settings.tsx (10)
frontend/javascripts/viewer/view/ai_jobs/run_ai_model/ai_image_segmentation_job_context.tsx (1)
useRunAiModelJobContext(276-282)frontend/javascripts/libs/react_hooks.ts (1)
useWkSelector(292-294)frontend/javascripts/types/api_types.ts (1)
APIDataLayer(114-114)frontend/javascripts/viewer/view/ai_jobs/components/collapsible_split_merger_evaluation_settings.tsx (2)
SplitMergerEvaluationSettings(4-9)CollapsibleSplitMergerEvaluationSettings(11-90)frontend/javascripts/theme.tsx (1)
ColorWKBlue(12-12)frontend/javascripts/admin/dataset/dataset_components.tsx (1)
getDatasetNameRules(68-85)frontend/javascripts/viewer/view/ai_jobs/utils.ts (3)
colorLayerMustNotBeUint24Rule(307-318)getBestFittingMagComparedToTrainingDS(45-87)isDatasetOrBoundingBoxTooSmall(116-139)frontend/javascripts/viewer/store.ts (1)
UserBoundingBox(108-110)frontend/javascripts/libs/utils.ts (1)
computeArrayFromBoundingBox(299-308)frontend/javascripts/viewer/view/ai_jobs/bounding_box_selector.tsx (1)
BoundingBoxSelector(15-38)
frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_job_context.tsx (8)
frontend/javascripts/types/api_types.ts (2)
APIAnnotation(589-591)APIDataset(243-246)frontend/javascripts/viewer/constants.ts (1)
Vector3(14-14)frontend/javascripts/viewer/store.ts (1)
UserBoundingBox(108-110)frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_model_selector.tsx (1)
AiTrainingTask(11-18)frontend/javascripts/viewer/model/accessors/tracing_accessor.ts (1)
getUserBoundingBoxesFromState(98-101)frontend/javascripts/admin/rest_api.ts (1)
getUnversionedAnnotationInformation(618-629)frontend/javascripts/admin/api/jobs.ts (3)
AiModelTrainingAnnotationSpecification(371-376)runInstanceModelTraining(402-407)runNeuronTraining(386-391)frontend/javascripts/viewer/model/actions/ui_actions.ts (1)
setAIJobDrawerStateAction(136-140)
frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_model_selector.tsx (2)
frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_job_context.tsx (1)
useAiTrainingJobContext(204-210)frontend/javascripts/theme.tsx (1)
ColorWKBlue(12-12)
🪛 GitHub Check: frontend-tests
frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_job_context.tsx
[failure] 139-139:
Type '{ annotationId: string; colorLayerName: string | undefined; segmentationLayerName: string | undefined; mag: Vector3 | undefined; }[]' is not assignable to type 'AiModelTrainingAnnotationSpecification[]'.
🪛 LanguageTool
docs/automation/ai_training.md
[style] ~7-~7: For conciseness, consider replacing this expression with an adverb.
Context: ...webknossos.org](https://webknossos.org) at the moment. If you want to set up on-premise ...
(AT_THE_MOMENT)
[grammar] ~46-~46: There might be a mistake here.
Context: ... Give your new model a descriptive name. * Comments: Add any comments or notes ab...
(QB_NEW_EN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-smoketest-push
- GitHub Check: backend-tests
🔇 Additional comments (6)
frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_job_context.tsx (3)
31-55: LGTM! Context type is now correctly aligned.The
handleStartAnalysisreturn type now correctly reflects the async implementation (Promise<void>), andhandleSelectionChangeproperly restricts updates to only the mutable fields viaPick<...>.
101-122: LGTM! Magnification reset logic is sound.The implementation correctly resets
magnificationwhen layers change, preventing stale mag values from mismatching the new layer's available magnifications.
154-166: LGTM! Job dispatch logic correctly branches by job type.The conditional dispatch calls the appropriate API based on
selectedJobType, passing the correct category and parameters for each training variant.frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_model_selector.tsx (3)
82-82: LGTM! Avatar.src now correctly receives a URL string.The previous nested
<img>element has been fixed, andAvatarnow receives the image URL directly viasrc={item.image}.
42-50: LGTM! Task selection handler is correct.The callback correctly checks for enabled items with a valid
jobTypebefore updating bothselectedTaskandselectedJobTypein the context.
66-79: LGTM! Visual feedback for selection and disabled states is clear.The styling correctly applies a blue border for the selected item and reduces opacity with a
not-allowedcursor for disabled items, providing clear visual feedback to users.
docs/automation/ai_training.md
Outdated
| WEBKNOSSOS allows you to train your own AI models for image segmentation. This feature is currently in early access. | ||
|
|
||
| <!-- Keep info in sync with docs/automation/ai_segmentation.md --> | ||
| !!!info |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Admonition syntax: verify “!!!info” vs “!!! info”.
Many MkDocs setups require a space after “!!!”. Please verify repo-wide usage and renderer behavior; adjust if needed.
🏁 Script executed:
#!/bin/bash
# Verify admonition style usage across docs
rg -nC1 '^\s*!!!info\b|^\s*!!!\s+info\b' docs
# Count occurrences by style
echo "Counts:"
printf "no-space: "; rg -n '^\s*!!!info\b' docs | wc -l
printf "with-space: "; rg -n '^\s*!!!\s+info\b' docs | wc -lLength of output: 2887
Unify admonition syntax to include a space after “!!!”
Repo-wide usage mixes !!!info (5 occurrences) and !!! info (4 occurrences). MkDocs Material requires a space; update all to !!! info for consistent rendering.
🤖 Prompt for AI Agents
In docs/automation/ai_training.md around line 6, the admonition uses "!!!info"
but MkDocs Material requires a space after "!!!"; update this line to "!!!
info". Also search the repo for other occurrences of "!!!info" and replace them
with "!!! info" (there are 5 occurrences of the no-space form) to ensure
consistent rendering across all markdown files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
♻️ Duplicate comments (2)
frontend/javascripts/viewer/view/ai_jobs/alignment/ai_alignment_job_context.tsx (1)
37-41: Critical issue from previous review remains unresolved: guard against missing color layers.The code still assumes
colorLayers[0]exists. If the dataset has no color layers,colorLayeris undefined and line 40 crashes when derivingselectedBoundingBox. Lines 48-51 validation and lines 53-67handleStartAnalysisalso fail to checkcolorLayerexistence before usingcolorLayer.name.Apply this diff to add guards:
const dataset = useWkSelector((state) => state.dataset); const annotationId = useWkSelector((state) => state.annotation.annotationId); const colorLayers = getColorLayers(dataset); - const colorLayer = colorLayers[0]; + const colorLayer = colorLayers[0] ?? null; - const selectedBoundingBox = getBoundingBoxesForLayers([colorLayer])[0]; + const selectedBoundingBox = colorLayer ? getBoundingBoxesForLayers([colorLayer])[0] : null;Update validation to include colorLayer check:
const areParametersValid = useMemo( - () => Boolean(selectedTask?.jobType && newDatasetName), - [selectedTask, newDatasetName], + () => Boolean(selectedTask?.jobType && newDatasetName && colorLayer), + [selectedTask, newDatasetName, colorLayer], );Short-circuit start when colorLayer is missing:
const handleStartAnalysis = useCallback(async () => { + if (!colorLayer) { + Toast.error("No color layer available. Please ensure the dataset has a color layer."); + return; + } try { await startAlignSectionsJob( dataset.id, colorLayer.name,docs/automation/ai_segmentation.md (1)
7-7: Unresolved from previous review: terminology mismatch with UI.The text still says "dialog" but the UI uses a "drawer". Update to match the implementation.
Apply this diff:
-You can launch the AI analysis dialog using the `AI Analysis` button in the toolbar at the top. This will open a dropdown menu with three options: +You can launch the AI analysis using the `AI Analysis` button in the toolbar at the top. This will open a dropdown menu with three options. Selecting an option opens the AI Jobs drawer:
🧹 Nitpick comments (2)
frontend/javascripts/viewer/view/ai_jobs/credit_information.tsx (2)
65-65: Remove unprofessional comment.The inline comment "This is a shitty way to do it" should be removed or reworded professionally. If the approach is temporary, use a standard TODO or FIXME marker.
Apply this diff:
- // sum all training volumes into a single bounding box - // This is a shitty way to do it, but it works for now. + // Sum all training volumes into a single synthetic bounding box for cost estimation const totalVolume = selectedAnnotations.reduce(
64-86: Synthetic cubic bounding box obscures actual training data distribution.The code sums volumes from all training annotations and creates a synthetic cube with side length
Math.cbrt(totalVolume). This approach loses information about the actual spatial distribution and shape of the training data. Consider whether the cost API can accept multiple bounding boxes or whether documentation should clarify this is an approximation.If the API supports it, pass multiple bounding boxes and sum costs. Otherwise, add a comment explaining the cubic approximation:
+ // Note: Training cost is estimated using a synthetic cubic bounding box with + // volume equal to the sum of all training annotation volumes. This approximates + // the actual spatial distribution for cost calculation purposes. const totalVolume = selectedAnnotations.reduce(
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
docs/automation/ai_segmentation.md(1 hunks)docs/automation/ai_training.md(1 hunks)docs/data/wkw.md(1 hunks)docs/proofreading/proofreading_tool.md(1 hunks)docs/sharing/dataset_sharing.md(1 hunks)frontend/javascripts/viewer/view/ai_jobs/alignment/ai_alignment_job_context.tsx(1 hunks)frontend/javascripts/viewer/view/ai_jobs/alignment/ai_alignment_model_selector.tsx(1 hunks)frontend/javascripts/viewer/view/ai_jobs/credit_information.tsx(1 hunks)frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_model_selector.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- docs/data/wkw.md
- docs/sharing/dataset_sharing.md
- docs/proofreading/proofreading_tool.md
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/javascripts/viewer/view/ai_jobs/alignment/ai_alignment_model_selector.tsx
- frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_model_selector.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-27T11:32:54.033Z
Learnt from: knollengewaechs
PR: scalableminds/webknossos#8796
File: frontend/javascripts/viewer/view/action-bar/ai_job_modals/forms/neuron_segmentation_form.tsx:68-77
Timestamp: 2025-08-27T11:32:54.033Z
Learning: In the webknossos frontend AI job forms, `datasetConfiguration.layers[colorLayer.name]` is assumed to always be non-null when accessing layer properties like `isInverted`. The selected color layer is guaranteed to exist in the dataset configuration.
Applied to files:
frontend/javascripts/viewer/view/ai_jobs/alignment/ai_alignment_job_context.tsx
🧬 Code graph analysis (2)
frontend/javascripts/viewer/view/ai_jobs/alignment/ai_alignment_job_context.tsx (7)
frontend/javascripts/viewer/view/ai_jobs/alignment/ai_alignment_model_selector.tsx (1)
AlignmentTask(11-18)frontend/javascripts/viewer/store.ts (1)
UserBoundingBox(108-110)frontend/javascripts/libs/react_hooks.ts (1)
useWkSelector(292-294)frontend/javascripts/viewer/view/ai_jobs/utils.ts (1)
getBoundingBoxesForLayers(30-40)frontend/javascripts/admin/api/jobs.ts (1)
startAlignSectionsJob(344-363)frontend/javascripts/viewer/model/actions/ui_actions.ts (1)
setAIJobDrawerStateAction(136-140)frontend/javascripts/libs/toast.tsx (1)
error(211-218)
frontend/javascripts/viewer/view/ai_jobs/credit_information.tsx (9)
frontend/javascripts/viewer/view/ai_jobs/run_ai_model/ai_image_segmentation_job_context.tsx (1)
useRunAiModelJobContext(276-282)frontend/javascripts/viewer/view/ai_jobs/alignment/ai_alignment_job_context.tsx (1)
useAlignmentJobContext(84-90)frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_job_context.tsx (1)
useAiTrainingJobContext(204-210)frontend/javascripts/libs/utils.ts (2)
computeVolumeFromBoundingBox(314-317)computeArrayFromBoundingBox(299-308)frontend/javascripts/viewer/store.ts (2)
UserBoundingBoxWithoutId(102-107)UserBoundingBox(108-110)frontend/javascripts/types/api_types.ts (1)
AiModel(833-844)frontend/javascripts/libs/react_hooks.ts (1)
useWkSelector(292-294)frontend/javascripts/admin/api/jobs.ts (2)
JobCreditCostInfo(80-86)getJobCreditCost(88-97)frontend/javascripts/libs/format_utils.ts (2)
formatVoxels(515-538)formatCreditsString(559-563)
🪛 GitHub Check: frontend-tests
frontend/javascripts/viewer/view/ai_jobs/credit_information.tsx
[failure] 46-46:
Type 'APIJobType | null | undefined' is not assignable to type 'APIJobType | null'.
🪛 LanguageTool
docs/automation/ai_segmentation.md
[grammar] ~18-~18: There might be a mistake here.
Context: ...and focused ion beam-SEM (FIB-SEM) data. * Mitochondria Detection: Run a pre-trai...
(QB_NEW_EN)
[grammar] ~31-~31: There might be a mistake here.
Context: ...et that the model will use for analysis. * Bounding Box: The region of interest t...
(QB_NEW_EN)
[style] ~47-~47: Consider using an alternative to strengthen your wording.
Context: ...analysis features in the future. If you want to work with us on an automated analysi...
(WANT_KEEN)
[grammar] ~47-~47: There might be a mistake here.
Context: ...ntact us](mailto:[email protected]). We would love to integrate analysis solu...
(QB_NEW_EN)
[style] ~52-~52: For conciseness, consider replacing this expression with an adverb.
Context: ...webknossos.org](https://webknossos.org) at the moment. If you want to set up on-premise ...
(AT_THE_MOMENT)
[grammar] ~52-~52: There might be a mistake here.
Context: ...(https://webknossos.org) at the moment. If you want to set up on-premise automat...
(QB_NEW_EN)
[grammar] ~53-~53: There might be a mistake here.
Context: ...act sales](mailto:[email protected]).
(QB_NEW_EN)
docs/automation/ai_training.md
[style] ~7-~7: For conciseness, consider replacing this expression with an adverb.
Context: ...webknossos.org](https://webknossos.org) at the moment. If you want to set up on-premise ...
(AT_THE_MOMENT)
[grammar] ~7-~7: There might be a mistake here.
Context: ...(https://webknossos.org) at the moment. If you want to set up on-premise automat...
(QB_NEW_EN)
[grammar] ~46-~46: There might be a mistake here.
Context: ... Give your new model a descriptive name. * Comments: Add any comments or notes ab...
(QB_NEW_EN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: backend-tests
- GitHub Check: build-smoketest-push
| AI Model Training is only available on [webknossos.org](https://webknossos.org) at the moment. | ||
| If you want to set up on-premise automated analysis at your institute/workplace, then [please contact sales](mailto:[email protected]). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tighten wording and use “on‑premises”.
Swap “at the moment” for “currently” (or drop it) and change “on-premise” to the correct “on‑premises” phrasing to keep the copy concise and polished.
🧰 Tools
🪛 LanguageTool
[style] ~7-~7: For conciseness, consider replacing this expression with an adverb.
Context: ...webknossos.org](https://webknossos.org) at the moment. If you want to set up on-premise ...
(AT_THE_MOMENT)
[grammar] ~7-~7: There might be a mistake here.
Context: ...(https://webknossos.org) at the moment. If you want to set up on-premise automat...
(QB_NEW_EN)
🤖 Prompt for AI Agents
In docs/automation/ai_training.md around lines 7 to 8, tighten the wording:
replace "at the moment" with "currently" (or remove it) and change "on-premise"
to the correct "on‑premises" phrasing; update the sentence to read concisely
(e.g., "AI Model Training is only available on webknossos.org currently. If you
want to set up on‑premises automated analysis at your institute/workplace,
please contact sales.") ensuring punctuation and link remain intact.
| To start a training, click on the `AI Analysis` button in the toolbar and select `Train AI model` from the dropdown menu. | ||
| This will open a dialog where you can configure and start your training job. | ||
| ### Select AI Training Task |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update instructions to match the drawer-based UI.
The AI job UI no longer opens a modal dialog—it now launches a drawer with a “Model Training” tab. Leaving the old wording misleads users following these steps. Please update this section to reference the drawer and tab so the docs stay accurate.
🤖 Prompt for AI Agents
In docs/automation/ai_training.md around lines 26 to 28, the text still refers
to a modal dialog for starting AI training; update it to reference the new
drawer-based UI and the “Model Training” tab. Change the sentence to instruct
users to click the AI Analysis button, select Train AI model, then use the
opened drawer and switch to the “Model Training” tab to configure and start the
job, ensuring wording clearly replaces “dialog” with “drawer” and mentions the
tab name.
frontend/javascripts/viewer/view/ai_jobs/credit_information.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
♻️ Duplicate comments (1)
frontend/javascripts/viewer/view/ai_jobs/run_ai_model/ai_image_segmentation_job_context.tsx (1)
164-191: Fix brittle custom vs. pre-trained model detection.The check
"trainingJob" in selectedModelreturnstruefor allAiModelobjects (sincetrainingJobis always present as a property), including pre-trained models wheretrainingJob: null. This causes pre-trained models to incorrectly route to the custom inference path.Apply this diff to check for non-null
trainingJob:- if ("trainingJob" in selectedModel!) { + if (selectedModel!.trainingJob != null) { // Custom models
🧹 Nitpick comments (2)
frontend/javascripts/viewer/view/ai_jobs/run_ai_model/ai_image_segmentation_job_context.tsx (2)
168-168: Remove unnecessary type assertion.The
idproperty is already typed asstringin theAiModelinterface, so theas stringassertion is redundant.Apply this diff:
- aiModelId: selectedModel!.id as string, + aiModelId: selectedModel!.id,
121-133: Consider a type guard function for better type safety.While the
areParametersValidcheck with early return prevents runtime issues, TypeScript cannot infer type narrowing fromlodash.every, requiring!operators throughout (lines 137, 161, 164, 168, 171, 198, 210, 216, 218). A custom type predicate would eliminate these assertions.Example:
function assertRequiredParameters( model: AiModel | Partial<AiModel> | null, jobType: APIJobType | null, bbox: UserBoundingBox | null, datasetName: string, layer: APIDataLayer | null, ): asserts model is AiModel | Partial<AiModel> { if (!model || !jobType || !bbox || !datasetName || !layer) { throw new Error("Missing required parameters"); } } // Then in handleStartAnalysis: try { assertRequiredParameters(selectedModel, selectedJobType, selectedBoundingBox, newDatasetName, selectedLayer); // TypeScript now knows these are non-null const boundingBox = computeArrayFromBoundingBox(selectedBoundingBox.boundingBox); // ... }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/javascripts/viewer/view/ai_jobs/credit_information.tsx(1 hunks)frontend/javascripts/viewer/view/ai_jobs/run_ai_model/ai_image_segmentation_job_context.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
frontend/javascripts/viewer/view/ai_jobs/run_ai_model/ai_image_segmentation_job_context.tsx (11)
frontend/javascripts/types/api_types.ts (2)
AiModel(833-844)APIDataLayer(114-114)frontend/javascripts/viewer/store.ts (1)
UserBoundingBox(108-110)frontend/javascripts/viewer/view/ai_jobs/components/collapsible_split_merger_evaluation_settings.tsx (1)
SplitMergerEvaluationSettings(4-9)frontend/javascripts/viewer/api/wk_dev.ts (1)
model(69-72)frontend/javascripts/libs/react_hooks.ts (1)
useWkSelector(292-294)frontend/javascripts/viewer/model/accessors/tracing_accessor.ts (2)
getUserBoundingBoxesFromState(98-101)getTaskBoundingBoxes(96-96)frontend/javascripts/libs/utils.ts (1)
computeArrayFromBoundingBox(299-308)frontend/javascripts/libs/toast.tsx (2)
messages(63-82)error(211-218)frontend/javascripts/viewer/model/accessors/skeletontracing_accessor.ts (1)
hasEmptyTrees(120-122)frontend/javascripts/admin/api/jobs.ts (6)
BaseCustomModelInferenceParameters(409-420)runCustomNeuronModelInferenceJob(427-432)runCustomInstanceModelInferenceJob(434-441)runPretrainedNeuronInferencelJob(207-253)runPretrainedMitochondriaInferenceJob(325-342)runPretrainedNucleiInferenceJob(191-205)frontend/javascripts/viewer/model/actions/ui_actions.ts (1)
setAIJobDrawerStateAction(136-140)
frontend/javascripts/viewer/view/ai_jobs/credit_information.tsx (9)
frontend/javascripts/viewer/view/ai_jobs/run_ai_model/ai_image_segmentation_job_context.tsx (1)
useRunAiModelJobContext(278-284)frontend/javascripts/viewer/view/ai_jobs/alignment/ai_alignment_job_context.tsx (1)
useAlignmentJobContext(84-90)frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_job_context.tsx (1)
useAiTrainingJobContext(204-210)frontend/javascripts/libs/utils.ts (2)
computeVolumeFromBoundingBox(314-317)computeArrayFromBoundingBox(299-308)frontend/javascripts/viewer/store.ts (2)
UserBoundingBoxWithoutId(102-107)UserBoundingBox(108-110)frontend/javascripts/types/api_types.ts (1)
AiModel(833-844)frontend/javascripts/libs/react_hooks.ts (1)
useWkSelector(292-294)frontend/javascripts/admin/api/jobs.ts (2)
JobCreditCostInfo(80-86)getJobCreditCost(88-97)frontend/javascripts/libs/format_utils.ts (2)
formatVoxels(515-538)formatCreditsString(559-563)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: frontend-tests
🔇 Additional comments (2)
frontend/javascripts/viewer/view/ai_jobs/run_ai_model/ai_image_segmentation_job_context.tsx (1)
177-188: VerifyseedGeneratorDistanceThresholdparameter placement.A previous reviewer indicated that
INFER_NEURONSshould receiveseedGeneratorDistanceThresholdwhileINFER_INSTANCESshould not. The current code passes this parameter only toINFER_INSTANCES. Please confirm whether the parameter assignment matches the backend API expectations.If the previous reviewer's guidance was correct, apply this diff:
switch (selectedJobType) { case APIJobType.INFER_NEURONS: await runCustomNeuronModelInferenceJob({ ...commonInferenceArgs, + seedGeneratorDistanceThreshold: seedGeneratorDistanceThreshold, }); break; case APIJobType.INFER_INSTANCES: await runCustomInstanceModelInferenceJob({ ...commonInferenceArgs, - seedGeneratorDistanceThreshold: seedGeneratorDistanceThreshold, }); break;frontend/javascripts/viewer/view/ai_jobs/credit_information.tsx (1)
118-129: Verify thatfeatures()values are stable.The
useMemohas an empty dependency array while reading fromfeatures(). If feature flags can change at runtime, the memoized mapping will be stale.If
features()can change at runtime, move the mapping outside the component or addfeatures()to the dependency array. Iffeatures()is guaranteed to be static for the component lifecycle, the current implementation is fine.
frontend/javascripts/viewer/view/ai_jobs/credit_information.tsx
Outdated
Show resolved
Hide resolved
frontend/javascripts/viewer/view/ai_jobs/credit_information.tsx
Outdated
Show resolved
Hide resolved
frontend/javascripts/viewer/view/ai_jobs/credit_information.tsx
Outdated
Show resolved
Hide resolved
| export const useRunAiModelJobContext = () => { | ||
| const context = useContext(RunAiModelJobContext); | ||
| if (context === undefined) { | ||
| throw new Error("useAiJobsContext must be used within a AiJobsContextProvider"); | ||
| } | ||
| return context; | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix inconsistent error message.
The error message references useAiJobsContext and AiJobsContextProvider, but the actual names are useRunAiModelJobContext and RunAiModelJobContextProvider.
Apply this diff:
if (context === undefined) {
- throw new Error("useAiJobsContext must be used within a AiJobsContextProvider");
+ throw new Error("useRunAiModelJobContext must be used within a RunAiModelJobContextProvider");
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const useRunAiModelJobContext = () => { | |
| const context = useContext(RunAiModelJobContext); | |
| if (context === undefined) { | |
| throw new Error("useAiJobsContext must be used within a AiJobsContextProvider"); | |
| } | |
| return context; | |
| }; | |
| export const useRunAiModelJobContext = () => { | |
| const context = useContext(RunAiModelJobContext); | |
| if (context === undefined) { | |
| throw new Error("useRunAiModelJobContext must be used within a RunAiModelJobContextProvider"); | |
| } | |
| return context; | |
| }; |
🤖 Prompt for AI Agents
In
frontend/javascripts/viewer/view/ai_jobs/run_ai_model/ai_image_segmentation_job_context.tsx
around lines 278 to 284, the thrown error uses the wrong hook and provider
names; update the error message to reference the correct identifiers:
useRunAiModelJobContext and RunAiModelJobContextProvider. Replace the existing
string in the thrown Error with one that reads something like
"useRunAiModelJobContext must be used within a RunAiModelJobContextProvider" so
the message accurately reflects the hook and provider names.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
frontend/javascripts/viewer/view/ai_jobs/credit_information.tsx (2)
43-43: Consider renaming for consistency.The variable
selectJobTypecould be renamed toselectedJobTypeto match the naming pattern used in other wrappers and the prop it's passed to (line 48).Apply this diff:
- const selectJobType = selectedTask?.jobType ?? null; + const selectedJobType = selectedTask?.jobType ?? null;And update line 48:
- selectedJobType={selectJobType} + selectedJobType={selectedJobType}
143-155: Use a stable key for the bounding box in the query.The query key currently uses the object reference
selectedBoundingBox?.boundingBox, which can cause unnecessary refetches when the object reference changes even if the content remains identical. Use a stable, serializable representation instead.Apply this diff:
const { data: jobCreditCostInfo, isFetching } = useQuery<JobCreditCostInfo>({ queryKey: [ "jobCreditCost", selectedJobType ?? "no-type", - selectedBoundingBox?.boundingBox ?? "no-bb", + selectedBoundingBox + ? computeArrayFromBoundingBox(selectedBoundingBox.boundingBox).join(",") + : "no-bb", ], queryFn: async () => await getJobCreditCost( selectedJobType!, computeArrayFromBoundingBox(selectedBoundingBox!.boundingBox), ), enabled: Boolean(selectedBoundingBox && selectedJobType), });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/javascripts/viewer/view/ai_jobs/credit_information.tsx(1 hunks)frontend/javascripts/viewer/view/ai_jobs/run_ai_model/ai_image_segmentation_job_context.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/javascripts/viewer/view/ai_jobs/run_ai_model/ai_image_segmentation_job_context.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/javascripts/viewer/view/ai_jobs/credit_information.tsx (8)
frontend/javascripts/viewer/view/ai_jobs/run_ai_model/ai_image_segmentation_job_context.tsx (1)
useRunAiModelJobContext(278-284)frontend/javascripts/viewer/view/ai_jobs/alignment/ai_alignment_job_context.tsx (1)
useAlignmentJobContext(84-90)frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_job_context.tsx (1)
useAiTrainingJobContext(204-210)frontend/javascripts/libs/utils.ts (2)
computeVolumeFromBoundingBox(314-317)computeArrayFromBoundingBox(299-308)frontend/javascripts/viewer/store.ts (1)
UserBoundingBox(108-110)frontend/javascripts/libs/react_hooks.ts (1)
useWkSelector(292-294)frontend/javascripts/admin/api/jobs.ts (2)
JobCreditCostInfo(80-86)getJobCreditCost(88-97)frontend/javascripts/libs/format_utils.ts (2)
formatVoxels(515-538)formatCreditsString(559-563)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-smoketest-push
- GitHub Check: backend-tests
This PR updates and refreshes the AI job UI. This is a complete re-write from scratch avoiding the one-form-to-rule-them-all approach for starting job that we had until now. Most components and logic for alignment, AI training and inference is separate und tuned for that usecase. They still share a similar structure and some components but otherwise are independent, which should let us expand them individually as needed in the future.
URL of deployed dev instance (used for testing):
Steps to test:
TODOs:
Issues:
(Please delete unneeded items, merge only when none are left open)
$PR_NUMBER.mdfile inunreleased_changesor use./tools/create-changelog-entry.py)